Skip to content

perf(codegen): inline charCodeAt and stop routing it through the dynamic bitwise helper (#7592) - #7601

Merged
proggeramlug merged 6 commits into
mainfrom
perf/7592-buildout-cadence
Aug 7, 2026
Merged

perf(codegen): inline charCodeAt and stop routing it through the dynamic bitwise helper (#7592)#7601
proggeramlug merged 6 commits into
mainfrom
perf/7592-buildout-cadence

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Closes the hash-phase half of #7592. #7594 and #7596 took build_out
from 57 s to ~5–6 s; this is the phase that was next-largest behind it.

I re-measured before touching anything, and two of the ticket's three

figures were stale

#7592's phase table was captured before #7594/#7596. Re-running the
instrumented workload on the pinned quiet host at v0.5.1338:

phase (500k records) #7592's table measured now
JSON.stringify 1,451 ms 267 ms
fnv1a 1,250 ms 1,207 ms

stringify had already been fixed by the GC pacing work — its old number was
GC pause charged to the phase it happened to land in. So there is no
stringify change in this PR
: at 267 ms it is 2.5% of the run, and touching
JSON.stringify's shape-template path for that would be risk without a
return. The fnv1a phase was real.

Leaf profile: 85% of the hash phase was FFI, not work

PERRY_DEBUG_SYMBOLS=1 + sample, isolated hash driver over the workload's
own 68 MB output, pinned quiet host:

symbol samples share
js_string_char_code_at 1919 31.5%
js_dynamic_bitxor 1892 31.0%
the JS loop itself 934 15.3%
js_string_index_to_i32 797 13.1%
js_get_string_pointer_unified 547 9.0%

h = (h ^ s.charCodeAt(i)) | 0 was making four opaque runtime calls per
character
over 68 million characters. Every one of those helpers is a
handful of instructions; the cost is the FFI boundary — and an opaque call on
a loop's critical path also blocks LICM, so the loop-invariant receiver unbox
and header loads could never be hoisted out.

Two independent defects

1. charCodeAt was not statically a Number. is_numeric_expr has arms
for locals, class fields, Math.*, typed-array reads and user functions, but
none for a String-method call. So h ^ s.charCodeAt(i) failed
expr/binary.rs's "both operands are statically primitive" test and routed
through js_dynamic_bitxor — the BigInt-aware helper — to compute an integer
xor. Fixed by teaching is_numeric_expr about the String methods that lower
to a raw double.

The admitted set is exactly charCodeAt, indexOf, lastIndexOf, search,
localeCompare — each verified against its lowering in
lower_string_method.rs (all sitofp of an i32 helper, or a helper
documented to return a plain f64). codePointAt is deliberately excluded:
it returns undefined out of range, which is a NaN-box tag, not a number —
claiming it numeric would hand a tagged value straight to an inline fadd.
at/charAt (strings) and startsWith/endsWith/includes (booleans) are
excluded for the same reason. The claim is gated on the receiver taking
codegen's proven-string routing, mirroring lower_call/property_get.rs's
condition exactly, so an any-typed receiver — which may be a user object
with its own charCodeAt — is never claimed.

2. charCodeAt itself had no inline path. Added one: a guard chain that
reproduces exactly what js_string_char_code_at + js_string_index_to_i32
compute, falling back to those same two calls for anything it cannot prove.

  • STRING_TAG receiver, handle ≥ 4096 (the runtime's is_valid_string_ptr
    magnitude check);
  • 0.0 <= index < 2^31-1 as ordered comparisons — a NaN-boxed index
    (a string, a bool, undefined, a real NaN) fails both and takes the slow
    arm where the full ToIntegerOrInfinity including user valueOf runs.
    This also makes the subsequent fptosi in-range, so it can never be poison;
  • utf16_len == byte_len — the runtime's own is_ascii_string predicate.
    Equality implies every byte is one UTF-16 code unit, hence every byte
    < 0x80, so no WTF-8 / lone-surrogate / astral payload can reach the byte
    load (Runtime split()/parseFloat() read past exact-sized slice allocations -> intermittent AV (c0000005) on hot paths #6085's bounded walk still owns those);
  • index < utf16_len.

No allocation and no call occur between the receiver re-read and the byte
load, so no collection can move the header underneath the fast path.

No new env knob: it rides PERRY_STATIC_STRING_LOWERING, the same gate the
sibling inline .length fast path uses (already keyed into the object cache
and the repsel knob-isolation harness).

The layout coupling is pinned, not assumed

The fast path reads StringHeader at offsets 0, 4 and 20. Offset 0 is
already an established contract (the inline .length load), but it was
resting on a doc comment. perry-codegen cannot depend on perry-runtime,
so the two sides get a const assertion at the struct definition
(STRING_HEADER_ABI_MATCHES_CODEGEN): reordering, resizing or padding
StringHeader now fails the runtime build, at the definition, instead of
silently miscompiling every .length and charCodeAt in every user program.

Result

Both arms built from one target dir with an identical package set
(-p perry -p perry-runtime-static -p perry-stdlib-static), run interleaved
on the pinned quiet host (perry-macos.local, load 1.5–2.3 throughout).
Output file cmp-identical and the reported hash identical on every row.

Isolated hash driver over the workload's own 68 MB output, 3 reps each,
three interleaved pairs:

ms ns/char
before 3,624 / 3,623 / 3,623 17.73 / 17.72 / 17.72
after 327 / 324 / 324 1.60 / 1.58 / 1.58

11.2x, and the run-to-run spread is under 0.2% on both arms.

Full pipeline, median of 2–3 interleaved pairs after warmup:

fnv1a total wall peak RSS
200k before 483 ms 3,233 ms 3.24 s 598.7 MB
200k after 43 ms 2,793 ms 2.80 s 598.7 MB
500k before 1,247 ms 11,495 ms 10.7 s 1,389.2 MB
500k after 111 ms 10,078 ms 1,389.2 MB

Peak RSS is unchanged (−0.1% at 200k, −0.002% at 500k) — this is a codegen
change with no allocation behaviour, so it adds nothing on top of #7594's and
#7596's RSS cost.

build_out is statistically identical across the arms (200k: 2,349 vs
2,351 ms; 500k: 9,127–9,192 vs 8,694–9,224, a ±5% band both sides). The first
500k pair I ran showed 8,461 vs 9,216 and I did not report it as a regression —
re-running showed it was a cold-page-cache first-run outlier on the base arm.

The post-fix leaf profile of the hash driver is a single symbol:

Sort by top of stack, same collapsed (when >= 5):
        perry_fn_fnv_only_ts__fnv1a32  (in fnv_sym_fix)        1530

Every runtime call is gone from the hot path.

The A/B above is at the #7594 base (08940c877), because that is where I
started before #7596 landed. That is the right base for this claim: the hash
phase allocates nothing and never touches the collector, so GC pacing cannot
move it — and indeed #7596 is a build_out change. The branch is rebased on
#7596 and re-measured; absolute post-rebase numbers are in the thread.

Also here

Validation

  • cargo test -p perry-runtime --lib --no-fail-fast — 1847 passed, 0 failed.
  • cargo test -p perry-codegen --lib --no-fail-fast — 676 passed, 0 failed.
  • New gap test test_gap_7592_char_code_at_inline.ts, byte-identical to the
    Node 26.5.1 oracle
    . It is deliberately adversarial to the fast path: ASCII
    heap string, SSO receiver, empty string, accented (multi-byte) payload,
    astral surrogate pair, lone surrogate, a full 0–255 binary-string round-trip
    (every byte ≥ 128 makes byte_len > utf16_len, so those must take the slow
    arm), out-of-range on both ends, ±Infinity, NaN, fractional and -0.5
    indices, string/bool/null/undefined/{valueOf} indices, the no-arg form, a
    nullable-string receiver, and the FNV-1a loop itself over each shape.
  • Sabotage-verified, five ways. Each mutation was applied, the suite run,
    and the mutation reverted:
    • disable the numeric claim → char_code_at_on_a_string_receiver_is_statically_numeric
      fails, the other two pass;
    • disable the inline path → ..._emits_the_inline_ascii_read fails, the other
      two pass;
    • widen the receiver gate to every receiver → the negative control
      char_code_at_on_an_unproven_receiver_keeps_the_runtime_lowering fails,
      the other two pass;
    • drop the tenured term from the nursery cap →
      nursery_cap_becomes_tenured_proportional_above_the_crossover fails;
    • drop the influx floor → all three nursery-cap tests fail.
      The first pass of the codegen assertions was itself vacuous — they matched
      @js_dynamic_bitxor anywhere in the module, which every module declares.
      They now match call double @js_dynamic_bitxor. The negative control caught
      that: it was passing on the declaration alone.
  • The StringHeader ABI assertion is sabotage-verified too — changing
    offset_of!(byte_len) from 4 to 8 fails the runtime build with
    evaluation panicked, and restoring it compiles.
  • Gates: raw_handle_debt.py 998/998, addr_class_inventory.py pass,
    class_id_collisions.py pass, check_file_size.sh pass (the inline lowering
    pushed lower_string_method.rs to 2100 lines, so it moved to a sibling
    module lower_string_method/char_code_at.rs), cargo fmt --all -- --check
    clean, cargo clippy on both changed crates introduces no new warning.

CI has a deep backlog, so the above is local validation — that is what this
PR is standing on.

Summary by CodeRabbit

  • Performance
    • Improved String.prototype.charCodeAt performance with a faster path for ASCII strings.
    • Enabled more efficient numeric handling for indexOf, lastIndexOf, search, and localeCompare.
  • Bug Fixes
    • Preserved correct behavior for non-ASCII strings, coercion, out-of-range indexes, and dynamic values.
    • Added safeguards to detect incompatible string layout changes.
  • Documentation
    • Updated benchmark findings to confirm full large-record processing and output compatibility, with performance remaining the primary gap.

Ralph Küpper added 5 commits August 7, 2026 23:06
…mic bitwise helper (#7592)

The FNV-1a phase of honest_bench's json_pipeline spent 85% of its leaf
profile in four opaque runtime calls per character over a 68 MB string.

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
Also splits the inline charCodeAt lowering into a sibling module to stay
under the 2000-line file cap.

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a4db834c-6536-4d21-86aa-9d9916e91b9f

📥 Commits

Reviewing files that changed from the base of the PR and between de52be2 and 4d1aed5.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • CLAUDE.md
  • Cargo.toml

📝 Walkthrough

Walkthrough

The PR adds guarded inline lowering for String.prototype.charCodeAt, static numeric recognition for selected string methods, StringHeader ABI checks, nursery-cap policy tests, regression coverage, updated benchmark and changelog documentation, and a package version update.

Changes

Inline charCodeAt lowering

Layer / File(s) Summary
String numeric inference
crates/perry-codegen/src/lower_call/mod.rs, crates/perry-codegen/src/type_analysis/numeric.rs
Codegen exposes property predicates for type analysis. Numeric analysis recognizes selected numeric-returning string methods for proven string receivers.
Guarded inline lowering and ABI contract
crates/perry-codegen/src/lower_string_method.rs, crates/perry-codegen/src/lower_string_method/char_code_at.rs, crates/perry-runtime/src/string/mod.rs
charCodeAt validates the receiver, index, ASCII content, and bounds before loading one byte directly. Unsupported cases use existing runtime calls. Compile-time checks verify StringHeader size and field offsets.
Regression coverage and benchmark records
crates/perry-codegen/src/type_analysis/numeric/tests.rs, test-files/test_gap_7592_char_code_at_inline.ts, changelog.d/7601-inline-char-code-at.md, benchmarks/honest_bench/workloads/1_json_pipeline/perry/json_pipeline.ts
Tests cover fast paths, fallbacks, dynamic receivers, related methods, and hash loops. Documentation records benchmark findings and resolved pipeline limitations.

Nursery-cap policy

Layer / File(s) Summary
Effective nursery-cap calculation and tests
crates/perry-runtime/src/gc/tenuring.rs
The effective cap uses the larger of influx-driven capacity and tenured reclaimable pressure divided by TENURED_EDEN_DIVISOR. Tests cover crossover behavior, maximum-term precedence, base-cap enforcement, and accessor wiring.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

  • PerryTS/perry#7592 — The PR updates the benchmark claims and adds the charCodeAt optimization described by this issue.

Suggested reviewers: thehypnoo

Sequence Diagram(s)

sequenceDiagram
  participant TypeAnalysis
  participant Codegen
  participant Runtime
  TypeAnalysis->>Codegen: classify proven String charCodeAt call as numeric
  Codegen->>Codegen: validate receiver, index, ASCII content, and bounds
  Codegen->>Runtime: call runtime charCodeAt for unsupported cases
  Codegen-->>Runtime: load ASCII byte for guarded fast-path cases
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: inlining charCodeAt and removing dynamic bitwise-helper routing.
Description check ✅ Passed The description is detailed and covers the change, issue linkage, implementation, results, tests, and validation, despite not using every template heading.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/7592-buildout-cadence

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Post-rebase absolute numbers (branch on #7596)

Rebuilt on 72fa5b473 (#7596) and re-run on the pinned quiet host, load
1.5–1.7. Output file cmp-identical to the pre-rebase base arm's output at
both sizes; hash b7e8a588 unchanged.

500k records, median of 3:

phase #7592's table (v0.5.1335) #7594 base this PR on #7596
readFileSync 89 ms 76 59
JSON.parse 742 659 650
build_out 57,409 9,191 3,970
JSON.stringify 1,451 266 273
writeFileSync 90 19 20
fnv1a 1,250 1,247 108
total 60,431 11,495 5,087
peak RSS 1,064 MB† 1,389 MB 1,414 MB

200k records, median of 3: build_out 1,397 ms, fnv1a 43 ms, total
1,842 ms, peak RSS 609.3 MB.

† the 1,064 MB row is from the original report on a different host (M1 Max)
and is not comparable to the pinned-host column. On one host, #7594
#7596 is +1.8% RSS and this PR is +0.0% — the interleaved A/B in the
description has both arms at 1,389.2 MB at 500k and 598.7 MB at 200k.

build_out and parse are now the whole remaining gap (4.6 s of 5.1 s). Both
are out of scope here; the design for the next build_out step (the double
copy #7596 names as its follow-up) is written up on #7592.

Broader correctness sweep

The is_numeric_expr change affects any charCodeAt / indexOf /
lastIndexOf / search / localeCompare on a statically-string receiver, so
I ran every gap test that uses one — 26 files, including
test_gap_atob_binary_high_bytes (the 0–255 binary-string round-trip),
test_gap_string_locale_2781_2845_2897, test_gap_sso_concat_string_index,
test_gap_repsel_canonical_str_locals, test_gap_object_string_wrappers,
test_gap_7232_i32_chain_double_rounding and test_gap_bigint.

26 pass, 0 diff, 0 node_fail, byte-identical to the Node 26.5.1 oracle.

@proggeramlug
proggeramlug merged commit a5fddd3 into main Aug 7, 2026
@proggeramlug
proggeramlug deleted the perf/7592-buildout-cadence branch August 7, 2026 21:46
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Audit before merge — verified end to end, merged as v0.5.1340

Reproduced the headline on my own probe — a runtime-built 33.5M-char ASCII
string (so nothing constant-folds), fnv1a loop, both arms my own builds:

user CPU ns/char IR census (the loop)
main 0.62 s 18.5 char_code_at=1, dynamic_bitxor=1
#7601 0.06 s 1.8 char_code_at=1 (fallback only), dynamic_bitxor=0

10.3×, identical output hash — consistent with the claimed 11.2× across
hosts. The IR census is the mechanism: the dynamic-bitwise call is gone and the
one remaining char_code_at call site is the guarded fallback, not the hot
path.

Sabotage re-verified, both halves:

  • Disabling the inline path → char_code_at_on_a_string_receiver_emits_the_inline_ascii_read
    red, everything else green. The negative-control hardening in the report is
    real — and worth naming as practice: the agent's own first assertions matched
    @js_dynamic_bitxor anywhere (every module declares it), which is a vacuous
    test, and its own negative control caught that before I could.
  • Dropping the tenured-proportional cap term →
    nursery_cap_becomes_tenured_proportional_above_the_crossover red. This
    closes the perf(gc): live-proportional collection budgets at both generations (#7592) #7596 coverage gap I flagged at its merge
    — and my first sabotage
    attempt missed because the code had been refactored into a pure function of
    its two inputs, which is itself the right fix: the sibling cap-scale test only
    looked like coverage because the test thread's old-gen is ~empty.

Root-dominance run because codegen changed (this is the class of change the
runtime probes cannot see): corpus 128/128 compiled, --moving-only 0
violations, 40/40 seeded caught
, --unrooted-allocas --moving-only 0. The
inline path's heap loads sit between guard and use with no interleaved
collection point, as the file's own comment states — and the corpus now
contains this lowering, so the claim is checked, not asserted.

Gates re-run here: perry-runtime 1,847/0, perry-codegen 675/0 restored
(674+1 under sabotage), gap test byte-identical to node (59 lines, exit 0),
addr_class, class_id, raw_handle_debt 998/998, check_file_size, fmt —
all clean.

Also verified the discipline items: codePointAt exclusion is correct (it
returns undefined out of range — a NaN-box tag, not a number, and admitting
it would be exactly the #7590 class of wrong-value bug). The stale-figure catch
(stringify 1,451 → 267 ms after #7594/#7596; GC pause had been charged to the
phase it landed in) is the fifth time this campaign that measure-first
invalidated a ticket number — and the deleted v0.5.29 "GAP NOTES" block was
re-probed before deletion, not just deleted.

Where #7592 stands after this

stage 500k total
filed 60,431 ms (97.6× bun)
#7594 latch ~12,100 ms
#7596 budgets ~5,800 ms
#7601 charCodeAt ~5,087 ms (~8× bun)

Remaining, per the phase split: build_out ~3,970 ms (the two-hop promotion —
promote-on-first-copy design is on the issue with the fixed-point trap named)
and JSON.parse ~742 ms. The issue stays open.

proggeramlug added a commit that referenced this pull request Aug 8, 2026
….1355) (#7641)

Unblocks lint's freshness gate, red since #7601 edited a fingerprinted harness file. Host steps M1 Max/64GB -> M1/8GB, stated in the artifact and the changelog. Three mechanical failures preceded it (missing undeclared deps, dirty-tree refusal, non-truncating fixed-path hyperfine exports); the last two are handled in the launcher, not in the fingerprinted run.sh.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant